Skip to content

First PR - Implements the Opentracing API#2

Merged
gpolaert merged 89 commits into
masterfrom
dev
May 3, 2017
Merged

First PR - Implements the Opentracing API#2
gpolaert merged 89 commits into
masterfrom
dev

Conversation

@gpolaert
Copy link
Copy Markdown
Contributor

@gpolaert gpolaert commented Apr 28, 2017

What does this PR do?

Introduce a new Tracer for the Java Instrumentation. Implementing the opentracing API

Who will it impact?

Raclette-team

Motivation

Propose a solution for instrumenting Java code

Testing Guidelines

Additional Notes

@gpolaert gpolaert merged commit 5f13cdb into master May 3, 2017
*
* @return true if root, false otherwise
*/
private boolean isRootSpan() {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two things:

  • We use the convention spanID == 0 to define the root. Was OT requiring TraceID == SpanID instead?

  • That won't work with distributed traces, where the "global root" can be from another host. In that case, you will have a "local root" instrumented here, but which won't follow this convention.
    So we should do it differently (either not relying on isRootSpan for the flush, or putting a "root" marker on the first span created).

*
* @param sampleRate a number [0,1] representing the rate ratio.
*/
public RateSampler(double sampleRate) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the statistic and sampling mechanisms below (Agent, backend) to work well, we have to put the applied sampleRate in the meta _sample_rate.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(let me fix this one, I need to work on my Java).

tylerbenson pushed a commit that referenced this pull request Jul 19, 2019
randomanderson added a commit that referenced this pull request Mar 26, 2021
^ This is the 1st commit message:

use env file to push version

^ The commit message #2 will be skipped:

^ upstream.env is in the current folder?
bantonsson pushed a commit that referenced this pull request Dec 6, 2021
Enable compilation with Java 11 for Jetty 11 instrumentation
DDJavierSantos added a commit that referenced this pull request Jun 23, 2023
# This is the 1st commit message:

started

Ready for test checks

spotless

fixing tests

Some more tests fixed

spotless

Some more fixes

spotless

Quota test added

cleanup

small optimization in HttpResponseHeaderModuleImpl

optimization

test fixes

spotless

# This is the commit message #2:

fix a import missing from master rebase
DDJavierSantos added a commit that referenced this pull request Aug 9, 2023
# This is the 1st commit message:

Removing statements that log sensitive information and exceptions since they would be sent over the wire by the telemetry logging API

Changing statements so they use placeholders instead of adding strings

# This is the commit message #2:

linter applied

Update dd-smoke-tests/jboss-modules/src/main/java/datadog/smoketest/jbossmodules/common/ServiceSupport.java

Co-authored-by: Santiago M. Mola <santiago.mola@datadoghq.com>

processing PR comments

Processing PR comments

This statement doesn't need the placeholder
@evanchooly evanchooly mentioned this pull request Sep 10, 2024
5 tasks
jordan-wong added a commit that referenced this pull request May 11, 2026
…lkit-generated)

This PR adds a new module `dd-java-agent/instrumentation/jedis/jedis-gen-3.0`
containing an alternative Jedis 3.0 instrumentation generated by the
APM instrumentation toolkit. Placed alongside existing `jedis-3.0`.

Module name follows dd-trace-java's instrumentation-naming convention:
{framework}-gen-{version} ending in the required version suffix.

Bundled with two test-framework bug fixes the toolkit's agent discovered
while iterating. They can be split into a separate PR if preferred — both
are principled and benefit all instrumentation tests.

## Module changes (jedis-gen-3.0/)

One agent-driven workflow run. Cost: $41.04, ~92 min, reviewer approved
first iteration.

Key choices the agent made:
- Instruments `redis.clients.jedis.Connection` (protocol layer — same target
  as existing `jedis-1.4`). Classloader matcher: `hasClassNamed(ProtocolCommand)`
  with `.and(not(hasClassNamed(CommandObject)))` to avoid clashing with jedis-4.0+
- `JedisClientDecorator` extends `DatabaseClientDecorator`
- Three Spock tests covering base + V0 + V1 naming schemas
- Muzzle: explicit `fail [,3.0.0)` + `pass [3.0.0,4.0.0)` + `skipVersions += "jedis-3.6.2"`
  (jedis-3.6.2 is a malformed Maven release with a literal `jedis-` prefix —
  same workaround used by existing `jedis-3.0` module on master)

## Test-framework fixes (instrumentation-testing/)

The agent's verbatim reasoning from apm_test diagnosis output:

### Fix #1 — Is.java: CharSequence comparison

> The Is matcher used `expected.equals(actual)`, but `String.equals()` requires
> the other object to also be a String. When span attributes return
> `UTF8BytesString` (a CharSequence implementation), the equality check fails
> even when text content is identical. Fix: Added fallback using
> `String.contentEquals(CharSequence)` in `Is.test()`.

Real bug: span attributes return `CharSequence` (`UTF8BytesString` in
production). The `Is<T>` matcher's `expected.equals(t)` is asymmetric — any
test using `is("redis.query")` on a span attribute would fail despite the
value being correct.

### Fix #2 — TagsMatcher.java: DD_SVC_SRC default + nullable ERROR_MSG

> The TagsMatcher.defaultTags() was missing the `_dd.svc_src` tag that the
> tracer automatically sets. Also, the `error(Class)` matcher didn't account
> for `error.message` tags when no specific message was expected. Both caused
> "Unexpected tags" assertion failures.

Two related issues. (1) `_dd.svc_src` parity gap: the Groovy framework
(`TagsAssert.groovy:158`) already handles this tag; JUnit 5 `TagsMatcher.java`
was missing it. (2) Asymmetric ERROR_MSG handling: `error(Class)` wasn't
adding any matcher for `error.message`, so spans containing one would fail
with "Unexpected tags".

## Verification

```
./gradlew :dd-java-agent:instrumentation:jedis:jedis-gen-3.0:check \\
          :dd-java-agent:instrumentation:jedis:jedis-gen-3.0:muzzle \\
          :dd-java-agent:instrumentation:jedis:jedis-gen-3.0:latestDepTest
BUILD SUCCESSFUL in 28s
```

Multi-JVM matrix not run locally; standard CI will cover that.

## Reviewer notes

- Framework fixes (Is.java, TagsMatcher.java) are independent of the
  jedis-gen-3.0 module. Can be split into a separate PR if preferred.
- Protocol-layer target (`Connection`, not `Client`) matches existing
  `jedis-1.4` pattern. A prior toolkit run (April 2026) incorrectly chose
  `Client` and produced zero spans; this run correctly chose `Connection`.
- Class names follow the project convention.
- The `skipVersions += "jedis-3.6.2"` workaround is copied from existing
  `jedis-3.0` module on master.

## Provenance

Generated by apm-instrumentation-toolkit (DataDog/apm-instrumentation-toolkit
branch eval/java). Research artifacts:
- docs/eval-research/runs/jedis3/attempt1/
- docs/eval-research/hypotheses/jedis3.md

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants